Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle anon lifetime arg being returned with named lifetime return type #45751

Merged
merged 2 commits into from
Nov 7, 2017

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Nov 4, 2017

When there's a lifetime mismatch between an argument with an anonymous
lifetime being returned in a method with a return type that has a named
lifetime, show specialized lifetime error pointing at argument with a
hint to give it an explicit lifetime matching the return type.

error[E0621]: explicit lifetime required in the type of `other`
  --> file2.rs:21:21
   |
17 |     fn bar(&self, other: Foo) -> Foo<'a> {
   |                   ----- consider changing the type of `other` to `Foo<'a>`
...
21 |                     other
   |                     ^^^^^ lifetime `'a` required

Follow up to #44124 and #42669. Fix #44684.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 4, 2017
@eddyb
Copy link
Member

eddyb commented Nov 4, 2017

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Nov 4, 2017
@estebank estebank force-pushed the issue-44684 branch 2 times, most recently from cfc9c2f to d3edb01 Compare November 5, 2017 03:38
When there's a lifetime mismatch between an argument with an anonymous
lifetime being returned in a method with a return type that has a named
lifetime, show specialized lifetime error pointing at argument with a
hint to give it an explicit lifetime matching the return type.

```
error[E0621]: explicit lifetime required in the type of `other`
  --> file2.rs:21:21
   |
17 |     fn bar(&self, other: Foo) -> Foo<'a> {
   |                   ----- consider changing the type of `other` to `Foo<'a>`
...
21 |                     other
   |                     ^^^^^ lifetime `'a` required
```

Follow up to rust-lang#44124 and rust-lang#42669.
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lookin' good. I left a few comments regarding the names of the tests but r=me otherwise.

/// It has been extended for the case of structs too.
///
/// Consider the example
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// the function arguments consist of a named region and an anonymous
// region and corresponds to `ConcreteFailure(..)`
/// Generate an error message for when the function arguments consist of a named region and
/// an anonymous region and corresponds to `ConcreteFailure(..)`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved, but could be better still =)

--> $DIR/ex3-both-anon-regions-earlybound-regions.rs:17:12
|
13 | fn baz<'a, 'b, T>(x: &mut Vec<&'a T>, y: &T)
| ----- -- these two types are declared with different lifetimes...
| - consider changing the type of `y` to `&'a T`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this test is misnamed. It is called "both-anon-regions", but, in fact, one of the regions is not anomymous. Maybe rename to something like ex2a-push-one-existing-name-early-bound.rs? (To be honest, the exN- naming scheme is not doing it for me, since it's not clear what list of tests these numbers derive from.)

}

impl<'a> Foo<'a> {
fn bar(&self, other: Foo) -> Foo<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe rename to ex1-return-one-existing-name-early-bound-in-struct or something?

@estebank
Copy link
Contributor Author

estebank commented Nov 7, 2017

@nikomatsakis updated

@bors r=nikomatsakis rollup

@bors
Copy link
Contributor

bors commented Nov 7, 2017

📌 Commit 805333b has been approved by nikomatsakis

kennytm added a commit to kennytm/rust that referenced this pull request Nov 7, 2017
Handle anon lifetime arg being returned with named lifetime return type

When there's a lifetime mismatch between an argument with an anonymous
lifetime being returned in a method with a return type that has a named
lifetime, show specialized lifetime error pointing at argument with a
hint to give it an explicit lifetime matching the return type.

```
error[E0621]: explicit lifetime required in the type of `other`
  --> file2.rs:21:21
   |
17 |     fn bar(&self, other: Foo) -> Foo<'a> {
   |                   ----- consider changing the type of `other` to `Foo<'a>`
...
21 |                     other
   |                     ^^^^^ lifetime `'a` required
```

Follow up to rust-lang#44124 and rust-lang#42669. Fix rust-lang#44684.
bors added a commit that referenced this pull request Nov 7, 2017
Rollup of 9 pull requests

- Successful merges: #45470, #45588, #45682, #45714, #45751, #45764, #45778, #45782, #45784
- Failed merges:
@bors bors merged commit 805333b into rust-lang:master Nov 7, 2017
@estebank estebank deleted the issue-44684 branch November 9, 2023 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants